Skip to content

Fix iptable rules in ubuntu 20 for bridge name#5318

Merged
yadvr merged 1 commit intoapache:4.15from
ravening:fix-iptable
Aug 19, 2021
Merged

Fix iptable rules in ubuntu 20 for bridge name#5318
yadvr merged 1 commit intoapache:4.15from
ravening:fix-iptable

Conversation

@ravening
Copy link
Member

@ravening ravening commented Aug 16, 2021

Description

In ubuntu20 the interface name contains @ synbol and
because of that even the iptable rules for brdige name
contains this symbol which causes ping issues.
Remove the @ symbol from iptable rule to fix the issue

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

This the output from ubuntu16

# bridge -o link show | awk '/master breth0-2999 / && !/^[0-9]+: vnet/ {print $2}'
eth0.2999

And iptable rule is

-A BF-breth0-2999 -m physdev --physdev-out eth0.2999 --physdev-is-bridged -j ACCEPT

This is the output from ubuntu20

# bridge -o link show | awk '/master brens3-2999 / && !/^[0-9]+: vnet/ {print $2}'
ens3.2999@ens3

And iptable rule is

-A BF-brens3-2999 -m physdev --physdev-out ens3.2999@ens3 --physdev-is-bridged -j ACCEPT

After the fix, below is the output from ubuntu20

# bridge -o link show | awk '/master brens3-1989 / && !/^[0-9]+: vnet/ {print $2}'
ens3.1989@ens3

iptable rule is

-A BF-brens3-1989 -m physdev --physdev-out ens3.1989 --physdev-is-bridged -j ACCEPT

How Has This Been Tested?

Fix iptable rules in ubuntu 20 for bridge name
22c38f8 
In ubuntu20 the interface name contains @ synbol and
because of that even the iptable rules for brdige name
contains this symbol which causes ping issues.
Remove the @ symbol from iptable rule to fix the issue
@ravening ravening changed the base branch from main to 4.15 August 16, 2021 09:36
yadvr
yadvr approved these changes Aug 16, 2021
View reviewed changes
Copy link
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM did not test it. @ravening should we migrate to using ip link (for ex. ip link show type bridge...)

@yadvr yadvr added this to the 4.15.2.0 milestone Aug 16, 2021
@yadvr yadvr requested review from GabrielBrascher and wido August 16, 2021 09:47
@yadvr yadvr requested a review from weizhouapache August 16, 2021 09:48
@ravening
Copy link
Member Author

ravening commented Aug 16, 2021

LGTM did not test it. @ravening should we migrate to using ip link (for ex. ip link show type bridge...)

@rhtyd I thought its better to add extra flags rather than changing the command itself. Im not sure if it will break something else

@yadvr
Copy link
Member

yadvr commented Aug 16, 2021

@ravening I tested your changes manually LGTM on Ubuntu 20.04/kvm host
@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Aug 16, 2021

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@weizhouapache
Copy link
Member

weizhouapache commented Aug 16, 2021

commands looks good. need to test the iptables rules.
@ravening have you tested if the security groups are correctly applied on host and effective on vms ?

@ravening
Copy link
Member Author

ravening commented Aug 16, 2021

commands looks good. need to test the iptables rules.
@ravening have you tested if the security groups are correctly applied on host and effective on vms ?

@weizhouapache yes.. this was the only one which was causing issue in ubuntu20. apart from that all other rules worked fine so far... if I find out more then I will either update this pr or create a new one

@blueorangutan
Copy link

blueorangutan commented Aug 16, 2021

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 882

@weizhouapache
Copy link
Member

weizhouapache commented Aug 16, 2021

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Aug 16, 2021

@weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

blueorangutan commented Aug 16, 2021

Trillian test result (tid-1657)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35433 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5318-t1657-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_projects.py
Smoke tests completed. 87 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

weizhouapache
weizhouapache approved these changes Aug 19, 2021
View reviewed changes
Copy link
Member

@weizhouapache weizhouapache left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

ps: this change has been merged into master in pr#5158
2d3f2fb

@yadvr yadvr merged commit 5ed3246 into apache:4.15 Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yadvr yadvr yadvr approved these changes

@weizhouapache weizhouapache weizhouapache approved these changes

@GabrielBrascher GabrielBrascher Awaiting requested review from GabrielBrascher

@wido wido Awaiting requested review from wido

Assignees

No one assigned

Labels

component:kvm component:security-group

Projects

None yet

Milestone

4.15.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@ravening @yadvr @blueorangutan @weizhouapache @DaanHoogland